Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(24.04): systemd and dependencies #274

Merged
merged 17 commits into from
Aug 30, 2024
Merged

feat(24.04): systemd and dependencies #274

merged 17 commits into from
Aug 30, 2024

Conversation

Meulengracht
Copy link
Member

@Meulengracht Meulengracht commented Jul 5, 2024 •
edited
Loading

Proposed changes

Add systemd slice and it's dependencies

Testing

Smoke testing mount

mkdir mount-root
chisel cut --root mount-root bash_bins coreutils_bins passwd_config base-files_base mount_bins
sudo chroot mount-root/
mkdir /proc
mount -t proc proc /proc
umount /proc

Smoke testing systemd

mkdir systemd-root
chisel cut --root systemd-root bash_bins coreutils_bins passwd_config base-files_base systemd_bins
sudo chroot systemd-root
mkdir /proc
mount -t proc proc /proc
systemctl enable [email protected]
systemctl disable [email protected]
systemctl preset-all
systemd --help
journalctl --update-catalog
umount /proc

Checklist

@github-actions GitHub Actions
Copy link

github-actions bot commented Jul 5, 2024 •
edited
Loading

Diff of dependencies:
None found.

@Meulengracht Meulengracht force-pushed the slices/systemd branch 5 times, most recently from e71981e to d2b90f5 Compare July 5, 2024 10:01
@Meulengracht Meulengracht mentioned this pull request Jul 5, 2024
Open
2 tasks
@cjdcordeiro cjdcordeiro requested a review from a team July 19, 2024 11:08
cjdcordeiro
cjdcordeiro requested changes Jul 19, 2024
View reviewed changes
Copy link
Collaborator

@cjdcordeiro cjdcordeiro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks.

the checks are failing (systemd related I believe).

I also couldn't make the mount_bins slice work...can you provide a test case for that?

@Meulengracht
Copy link
Member Author

thanks.

the checks are failing (systemd related I believe).

I also couldn't make the mount_bins slice work...can you provide a test case for that?

I updated the description in the PR with a testing case

cjdcordeiro
cjdcordeiro requested changes Jul 24, 2024
View reviewed changes
Copy link
Collaborator

@cjdcordeiro cjdcordeiro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. could you please put that into an integration test?

@Meulengracht Meulengracht force-pushed the slices/systemd branch 2 times, most recently from 9d783ef to 8081645 Compare August 1, 2024 10:04
@cjdcordeiro cjdcordeiro requested a review from a team August 5, 2024 08:30
@cjdcordeiro cjdcordeiro added the Priority Look at me first label Aug 5, 2024
@cjdcordeiro cjdcordeiro self-assigned this Aug 5, 2024
zhijie-yang
zhijie-yang approved these changes Aug 5, 2024
View reviewed changes
Copy link
Collaborator

@zhijie-yang zhijie-yang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. One thing worth noticing: adding the --cap-add SYS_ADMIN and --security-opt apparmor=confined would make the test runner vulnerable to malicious test cases. We must carefully review the spread tests from external contributors before we let them run.

rebornplusplus
rebornplusplus requested changes Aug 6, 2024
View reviewed changes
Copy link
Member

@rebornplusplus rebornplusplus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice slices! Nice tests too!

I left a few comments below -- mostly about the systemd slices, with a few nitpicks here and there. Please let me know what you think about those.

slices/libargon2-1.yaml Outdated Show resolved Hide resolved
slices/libdevmapper1.02.1.yaml Outdated Show resolved Hide resolved
slices/systemd.yaml Outdated Show resolved Hide resolved
slices/systemd.yaml Outdated Show resolved Hide resolved
slices/systemd.yaml Outdated Show resolved Hide resolved
slices/systemd.yaml Outdated Show resolved Hide resolved
slices/systemd.yaml Outdated Show resolved Hide resolved
slices/systemd.yaml Outdated Show resolved Hide resolved
slices/systemd.yaml Outdated Show resolved Hide resolved
slices/systemd.yaml Outdated Show resolved Hide resolved
@cjdcordeiro cjdcordeiro mentioned this pull request Aug 19, 2024
Merged
2 tasks
@Meulengracht
Copy link
Member Author

@rebornplusplus this PR is ready for review

rebornplusplus
rebornplusplus reviewed Aug 21, 2024
View reviewed changes
Copy link
Member

@rebornplusplus rebornplusplus left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me overall. Only left a few nitpicks and questions.

Splendid job slicing the systemd package! (Other packages too) Took me a long while to review those systemd slices, I wonder what it must have taken you to write them in the first place.

slices/systemd.yaml Show resolved Hide resolved
slices/systemd.yaml Outdated Show resolved Hide resolved
slices/systemd.yaml Outdated Show resolved Hide resolved
slices/systemd.yaml Show resolved Hide resolved
slices/systemd.yaml Show resolved Hide resolved
@cjdcordeiro cjdcordeiro mentioned this pull request Aug 21, 2024
Merged
3 tasks
@Meulengracht
Copy link
Member Author

@rebornplusplus @cjdcordeiro tests are now running with rebased on master with the lxd backend, the tests are restored, can we give this a review again?

rebornplusplus
rebornplusplus approved these changes Aug 28, 2024
View reviewed changes
Copy link
Member

@rebornplusplus rebornplusplus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, thank you very much!

Let's settle on the nice.conf file in that thread however, before merging.

@Meulengracht
Copy link
Member Author

Meulengracht commented Aug 28, 2024 •
edited
Loading

Looks good to me, thank you very much!

Let's settle on the nice.conf file in that thread however, before merging.

I found the file and added it in my last commit! Thanks for the reviews!

It seems the file may have been moved.

EDIT: added the correct path :-)

@Meulengracht Meulengracht mentioned this pull request Aug 28, 2024
Open
3 tasks
@Meulengracht
slices: add initial version of systemd and friends.
b73785b 
Currently systemd is not working as it's lacking the catalog update functionality, blocked by maybe lack of features in Starlark/Chisel
@Meulengracht
slices/systemd: rely on initialization services upon first boot of sy…
047ed63 
…stemd
@Meulengracht
slices: add systemd dbus interfaces
b87fe89
@Meulengracht
slices/systemd: fix linting issues and build issues on non-amd64 plat…
c76d0c8 
…forms
@Meulengracht
slices/systemd: for some reason nice.conf does not exist anymore
6c6f8ba
@Meulengracht
tests/spread/integration: add tests for systemd and mount
a7a7308
@Meulengracht
tests: change tests to not require a priveliged container
0c4a327
@Meulengracht
slices: refactor systemd slices
3984cde
@Meulengracht
slices: add full version for libdevmapper
ac1f3b3
@Meulengracht
slices: move arch inline where we can
e95133e
@Meulengracht
slices: reorder essential in bins
407f969
@Meulengracht
slices/systemd: rename dbus slice
9b09985
@Meulengracht
slices/systemd: update namings based on review feedback
8b61816
@Meulengracht
Revert "tests: change tests to not require a priveliged container"
541afc2 
This reverts commit ce546ec.
@Meulengracht
spread: remove docker changes
7aab657
@Meulengracht
slices: add the missing nice.conf
34029a9
@Meulengracht
slices/systemd: sort correctly
c0b09f3
@cjdcordeiro cjdcordeiro merged commit c8b6fde into canonical:ubuntu-24.04 Aug 30, 2024
14 checks passed
vpa1977 added a commit to vpa1977/chisel-releases that referenced this pull request Aug 30, 2024
@vpa1977
fix(24.04): reference correct libmount1 copyright
b07109c 
canonical#274
referenced libssecomp2 copyright instead of libmount1.
Use libmount1 copyright.
@vpa1977 vpa1977 mentioned this pull request Aug 30, 2024
Merged
3 tasks
cjdcordeiro pushed a commit that referenced this pull request Aug 30, 2024
@vpa1977
fix(24.04): reference correct libmount1 copyright (#327)
c4497d5 
#274
referenced libssecomp2 copyright instead of libmount1.
Use libmount1 copyright.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zhijie-yang zhijie-yang zhijie-yang approved these changes

@rebornplusplus rebornplusplus rebornplusplus approved these changes

@cjdcordeiro cjdcordeiro cjdcordeiro approved these changes

Assignees

@cjdcordeiro cjdcordeiro

Labels
Priority Look at me first
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Meulengracht @cjdcordeiro @rebornplusplus @zhijie-yang